Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Harmonize focus-visible styles for vue-navigation item with none-vue navigation item #5022

Merged
merged 1 commit into from
Jan 4, 2024
Merged

Harmonize focus-visible styles for vue-navigation item with none-vue navigation item #5022

merged 1 commit into from
Jan 4, 2024

Conversation

JuliaKirschenheuter
Copy link
Contributor

☑️ Resolves

Before
Vue-navigation

Peek 2024-01-03 15-44

Unchanged non-vue navigation

Peek 2024-01-03 15-31

After
Vue-navigation

Peek 2024-01-03 15-30

Vue-navigation Unchanged non-vue navigation
Screenshot from 2024-01-03 15-31-17 Screenshot from 2024-01-03 15-31-12

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@JuliaKirschenheuter JuliaKirschenheuter added the 3. to review Waiting for reviews label Jan 3, 2024
@JuliaKirschenheuter JuliaKirschenheuter self-assigned this Jan 3, 2024
@JuliaKirschenheuter JuliaKirschenheuter mentioned this pull request Jan 3, 2024
Merged
4 tasks
@emoral435 emoral435 added enhancement New feature or request accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Jan 3, 2024
emoral435
emoral435 approved these changes Jan 3, 2024
View reviewed changes
Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on my local dev env - LGTM :) really nice change, good for a11y

emoral435
emoral435 suggested changes Jan 3, 2024
View reviewed changes
Copy link
Contributor

@emoral435 emoral435 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I found something that looks broken - this is for the subitems beneath the shared files section here:
image
the border-radius may have to be increased, or due to the shift padding/margin on the submenu files in shared (and also probably the favorites section...) this has to be changed before it gets merged :) sorry to be so nitpicky ;(

@JuliaKirschenheuter
Copy link
Contributor Author

Actually, I found something that looks broken - this is for the subitems beneath the shared files section here: the border-radius may have to be increased, or due to the shift padding/margin on the submenu files in shared (and also probably the favorites section...) this has to be changed before it gets merged :) sorry to be so nitpicky ;(

Yeah, I've seen it too. But it is actually another issue (which is not relevant for a11y) and is present on master:

Screenshot from 2024-01-04 09-16-08

@emoral435
Copy link
Contributor

Yeah, I've seen it too. But it is actually another issue (which is not relevant for a11y) and is present on master:
I see then. this is all good to me then!

@emoral435 emoral435 merged commit fa38cf9 into master Jan 4, 2024
15 checks passed
@emoral435 emoral435 deleted the fix/41699-change_styles_for_focused_items_in_sidebar branch January 4, 2024 17:20
@Pytal Pytal mentioned this pull request Jan 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emoral435 emoral435 emoral435 approved these changes

@Pytal Pytal Pytal approved these changes

@ShGKme ShGKme Awaiting requested review from ShGKme

@szaimen szaimen Awaiting requested review from szaimen

Assignees

@JuliaKirschenheuter JuliaKirschenheuter

Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JuliaKirschenheuter @emoral435 @Pytal